Skip to content

Added ability to ignore empty & whitespace strings#73

Closed
ludvigeriksson wants to merge 1 commit intoFlineDev:stablefrom
ludvigeriksson:stable
Closed

Added ability to ignore empty & whitespace strings#73
ludvigeriksson wants to merge 1 commit intoFlineDev:stablefrom
ludvigeriksson:stable

Conversation

@ludvigeriksson
Copy link
Copy Markdown
Contributor

When having empty strings (consisting of whitespace only) in the storyboard there is now an option to not add them to the strings file. For example, you might have a whitespace only string on the back button in the navigation bar.

This adds the command line option i (long version ignore-empty-strings), which enables the option.

@Jeehut
Copy link
Copy Markdown
Member

Jeehut commented Oct 16, 2017

Hi @ludvigeriksson and thank you for this pull request.

Could you please add the following so I can merge this:

  • Documentation for this new option in the same style as existing options
  • A unit test that proves that this options works so we don't break it in the future

for newTranslation in newTranslations {
// skip keys marked for ignore
guard !newTranslation.value.containsAny(of: ignores) else { continue }
if ignoreEmptyStrings && newTranslation.value.trimmingCharacters(in: .whitespaces).isEmpty { continue }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the .isBlank method from HandySwift instead of .trimmingCharacters(in: .whitespaces).isEmpty.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I'd actually prefer a guard here as the continue is a jump command and those should all be part of guards. I know that we have a negation then, but I prefer guards nevertheless.

@Jeehut
Copy link
Copy Markdown
Member

Jeehut commented Jan 9, 2018

@ludvigeriksson Any idea when you will have time to complete your feature?

@Jeehut
Copy link
Copy Markdown
Member

Jeehut commented Feb 5, 2018

Closing this due to inactivity. Feel free to open a new PR if you have more time again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants